Add Alexa skill integration and preserve env vars #52
Conversation
Summary of ChangesHello @claudio-pi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application by adding a new Alexa skill integration, enabling users to send messages to Telegram via voice commands. It includes robust security validations for incoming Alexa requests to ensure authenticity and integrity. Additionally, a critical fix was implemented to prevent the unintended deletion of unmanaged environment variables, improving the stability and configurability of the system. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature, Alexa skill integration, and also fixes a bug related to preserving environment variables. All identified issues, including a critical security flaw where signature verification is bypassed if the cryptography library is missing, and logging of sensitive information (full request bodies, PII, and potentially sensitive headers) within the new Alexa endpoint, remain paramount concerns. Additionally, suggestions for improving code quality and consistency in lib/server.py, such as simplifying header access and modernizing string formatting for logs, are still relevant.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 3 files
Confidence score: 1/5
- Critical auth bypass in
lib/server.py: whencryptographyis missing, signature verification returns True, allowing spoofed Alexa requests - Several user-impacting issues (missing
TELEGRAM_CHAT_IDcausing crashes, logging full request bodies leaking PII) make this a high-risk merge despite limited file scope - Pay close attention to
lib/server.py- authentication fallback, config guarding, and sensitive logging
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="lib/server.py">
<violation number="1" location="lib/server.py:261">
P2: Logging the full Alexa request body exposes PII and sensitive user data to the logs. This can lead to data leakage and should be removed or limited to non-sensitive fields only.</violation>
<violation number="2" location="lib/server.py:361">
P1: `int(TELEGRAM_CHAT_ID)` will raise `ValueError` if `TELEGRAM_CHAT_ID` is not set (empty string default). Add a guard at the start of `_handle_alexa` or `_enqueue_alexa_message` to reject early when `TELEGRAM_CHAT_ID` is unconfigured.</violation>
<violation number="3" location="lib/server.py:428">
P0: Critical authentication bypass: When the `cryptography` library is not installed, this returns `True`, effectively skipping signature verification for Alexa requests. An attacker could spoof Alexa requests by simply providing a valid-looking Amazon certificate URL and a recent timestamp. The application should fail securely by returning `False` if proper verification cannot be performed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces Alexa skill integration. A critical security vulnerability exists in the Alexa endpoint's request validation: signature verification is silently bypassed if the cryptography library is not installed, which is the default. This allows unauthorized attackers to send messages and potentially execute arbitrary commands via LLM tools. It is strongly recommended to make signature verification mandatory and update installation scripts to include the necessary dependencies, removing the insecure fallback. Additionally, there are important suggestions to enhance the robustness and security of the Alexa request validation logic, including proper header handling, consistent import usage, and comprehensive certificate validity checks.
claudio-pi
force-pushed
the
alexa-integration
branch
from
11512e6 to
f27c91d
Compare
|
All review comments addressed in 99ccaa7:
Tests pass (9/9). |
|
All review comments addressed in 99ccaa7. P0 auth bypass fixed (fail-secure), PII logging removed, TELEGRAM_CHAT_ID guard added, cert expiry validation added, deprecated SHA-1 header dropped, code cleanup done. Tests pass (9/9). |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="lib/server.py">
<violation number="1" location="lib/server.py:104">
P2: Calling the webhook handler without a timeout can leave the per-chat queue thread blocked forever if Claude hangs. This will stall all later messages for that chat and can prevent graceful shutdown. Reintroduce a bounded timeout and timeout cleanup logic.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
claudio-pi
force-pushed
the
alexa-integration
branch
from
7ce2c05 to
eabbe0f
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces Alexa skill integration, allowing voice commands to be relayed to Claude Code via Telegram. It also includes a fix for preserving unmanaged environment variables during configuration saves. The new Alexa endpoint incorporates robust security measures, including signature verification, certificate chain validation, and timestamp checks. The changes enhance the system's functionality and security posture, with clear documentation for the new feature and its associated risks.
Alexa requests are received at /alexa, validated (cert chain, timestamp, skill ID), and relayed as synthetic Telegram webhooks. Includes full cryptographic signature verification when the cryptography library is available, with graceful degradation to header+timestamp checks. Also adds a progressive fallback in telegram_send_message: if reply_to fails (e.g. for synthetic Alexa message IDs), retries without it.
Add critical security warning in the CAUTION section explaining the higher risk profile of the Alexa integration vs Telegram-only setup. Add full Alexa section under Customization with how-it-works flow, step-by-step setup instructions, and security considerations. Add ALEXA_SKILL_ID to the environment variables reference.
…cert validation - Return False when cryptography library is missing instead of bypassing signature verification (P0 security fix) - Remove full request/response body logging to prevent PII leakage - Add TELEGRAM_CHAT_ID guard in _enqueue_alexa_message - Validate certificate expiry before caching - Use only Signature-256 header (SHA-256), drop deprecated SHA-1 fallback - Use top-level urllib.parse instead of local import - Remove unused hashlib import - Replace raw header dump with safe presence-only log
… behavior The Alexa voice query prefix and quoting is now applied in _enqueue_alexa_message so the formatting is deterministic.
Reintroduces WEBHOOK_TIMEOUT (600s) on proc.communicate() that was accidentally removed in 99ccaa7. Without it, a hung Claude process blocks the per-chat queue thread forever. Now properly kills the process group (SIGTERM then SIGKILL) on timeout.
Extract request.locale from Alexa requests and use it to serve responses in the user's language. Supports Spanish and English with English as fallback for unsupported locales.
…e required, not optional
claudio-pi
force-pushed
the
alexa-integration
branch
from
eabbe0f to
55d16da
Compare
…on end Instead of sending each Alexa utterance as a separate webhook, buffer messages per session and flush them as a single transcript when the user says goodbye or the session times out. Includes expanded sample utterances with carrier phrases to reduce Alexa catch-all escapes.
Add carrier phrase variants in both Spanish and English to reduce Alexa escaping to native intents. Covers action verbs, interrogatives, continuations, and polite forms.
claudio-pi
force-pushed
the
alexa-integration
branch
from
c20362d to
50cd3ee
Compare
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="lib/server.py">
<violation number="1" location="lib/server.py:431">
P2: Single-message Alexa transcripts are sent verbatim, so voice input that starts with a slash (e.g., "/opus") will now trigger Telegram slash commands. Keep a non-command prefix/formatting to avoid accidental or unauthorized command execution from Alexa.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
2 participants
Summary
/alexaendpoint)claudio_save_envsilently deleting unmanaged env vars (likeHASS_TOKEN,ALEXA_SKILL_ID)Security review (CISO)
cryptographylib when availableTest plan
/modelcommand, verifyHASS_TOKENandALEXA_SKILL_IDsurvive inservice.envcryptographyinstalled — verify fallback validation works/alexarejects requests with invalid/missing skill ID